Add basic support for recursive TypeVar defaults (PEP 696) #16878
Conversation
This comment has been minimized.
This comment has been minimized.
| self.recursive_tvar_guard[tvar_id] = None | ||
| repl = repl.accept(self) | ||
| if isinstance(repl, TypeVarType): | ||
| repl.default = repl.default.accept(self) |
There was a problem hiding this comment.
Shouldn't we .copy_modified() here?
There was a problem hiding this comment.
That doesn't work, unfortunately. This here is used to link nested TypeVarTypes so if it's changed / evaluated in one location the other stays in sync.
An example from the tests
T1 = TypeVar("T1", default=str)
T2 = TypeVar("T2", default=T1)
class ClassD1(Generic[T1, T2]): ...
k = ClassD1()
reveal_type(k) # should be `ClassD1[str, str]`For the first pass the type variables are as follows
T1`1 = str
T2`2 = T1`1 = strThe section here now replaces the default for T2 with the tbd of the default of T1:
T1`1 = str
T2`2 = <result of T1`1>--
With .copy_modified it would be a copy of the T1'1 thus when it's evaluated it's not used for T2. The test would still return
ClassD1[str, T1`1 = str]At least that's how I understand it 😅
There was a problem hiding this comment.
Looked at it more closely. It's basically as said above. In particular, it's called in freshend_function_type_vars. We enter this function with linked TypeVars
callee.variables[0] == callee.variables[1].default # TrueThen create new ones for each and link them again in expand_type, so that
tvs[0] == tvs[1].default # True
# and by extension
fresh.variables[0] == fresh.variables[1].default # TrueLines 119 to 130 in 517f5ae
mypy/semanal.py
Outdated
| @@ -1954,6 +1954,15 @@ class Foo(Bar, Generic[T]): ... | |||
| del base_type_exprs[i] | |||
| tvar_defs: list[TypeVarLikeType] = [] | |||
| for name, tvar_expr in declared_tvars: | |||
| tvar_expr_default = tvar_expr.default | |||
| if isinstance(tvar_expr_default, UnboundType): | |||
| # Assumption here is that the names cannot be duplicated | |||
There was a problem hiding this comment.
Unfortunately that's not a safe assumption
There was a problem hiding this comment.
Do you have an example I could try? Mypy will already complain about this here and reject the second TypeVar call.
T1 = TypeVar("T1", default=int)
T1 = TypeVar("T1", default=str) # errorerror: Cannot redefine "T1" as a type variable
error: Invalid assignment target
error: "int" not callable
There was a problem hiding this comment.
I think you have to import the other TypeVar from a different module.
There was a problem hiding this comment.
That results in the same error message. The second TypeVar call would just be ignored. The logic here would still as it only compares the actual TypeVar name type_var_name = fullname.rpartition(".")[2]. Not the complete fullname.
E.g. the comparison is with T1 not __main__.T1.
There was a problem hiding this comment.
I thought something like this might fail:
[case testTypeVarDefaultsMultipleFiles]
from typing import Generic, TypeVar
from file2 import T as T2
T = TypeVar('T')
class Gen(Generic[T, T2]):
pass
def func(a: Gen, b: Gen[str], c: Gen[str, str]) -> None:
reveal_type(a) # N: Revealed type is "__main__.Gen[Any, builtins.int]"
reveal_type(b) # N: Revealed type is "__main__.Gen[builtins.str, builtins.int]"
reveal_type(c) # N: Revealed type is "__main__.Gen[builtins.str, builtins.str]"
[file file2.py]
from typing import TypeVar
T = TypeVar('T', default=int)
But this passes as expected, and I couldn't find a variation that fails. So I suppose this is fine.
There was a problem hiding this comment.
This variation fails:
[case testTypeVarDefaultsMultipleFiles]
from typing import Generic, TypeVar
from file2 import T as T2
T = TypeVar('T', default=T2)
class Gen(Generic[T2, T]):
pass
def func(a: Gen, b: Gen[str], c: Gen[str, float]) -> None:
reveal_type(a) # N: Revealed type is "__main__.Gen[builtins.int, builtins.int]"
reveal_type(b) # N: Revealed type is "__main__.Gen[builtins.str, builtins.str]"
reveal_type(c) # N: Revealed type is "__main__.Gen[builtins.str, builtins.float]"
[file file2.py]
from typing import TypeVar
T = TypeVar('T', default=int)
Expected:
main:11: note: Revealed type is "__main__.Gen[builtins.int, builtins.int]" (diff)
main:12: note: Revealed type is "__main__.Gen[builtins.str, builtins.str]" (diff)
main:13: note: Revealed type is "__main__.Gen[builtins.str, builtins.float]"
Actual:
main:11: note: Revealed type is "__main__.Gen[builtins.int, T2?]" (diff)
main:12: note: Revealed type is "__main__.Gen[builtins.str, T2?]" (diff)
main:13: note: Revealed type is "__main__.Gen[builtins.str, builtins.float]"
Not code I'd expect anyone to actually write, but it does seem like it should be legal.
There was a problem hiding this comment.
Thanks for the test, that helped! Tbh I could also have though about this earlier but it's actually quite simple to add a name lookup here 😅
|
Diff from mypy_primer, showing the effect of this PR on open source code: steam.py (https://github.com/Gobot1234/steam.py)
+ steam/ext/commands/commands.py:851: error: Overloaded function implementation does not accept all possible arguments of signature 3 [misc]
+ steam/ext/commands/commands.py:851: error: Overloaded function implementation cannot produce return type of signature 3 [misc]
|
Ref: #14851